Skip to content

#243: Do not warn for unmapped source properties on maps#244

Open
thunderhook wants to merge 4 commits into
mainfrom
243
Open

#243: Do not warn for unmapped source properties on maps#244
thunderhook wants to merge 4 commits into
mainfrom
243

Conversation

@thunderhook
Copy link
Copy Markdown
Contributor

No description provided.

@hduelme
Copy link
Copy Markdown
Contributor

hduelme commented May 2, 2026

@thunderhook good catch. Your implementation works for a single source parameter of type Map<String, ?>, but Map<Integer, ?> still disables the warnings. Also, Map handling is missing Map-as-normal-source detection that MapStruct performs.

@thunderhook
Copy link
Copy Markdown
Contributor Author

Thanks @hduelme !

I've adapted the check and:

  • fixed it when it comes to more than one parameter (e.g. using @Context)
  • enabled the warning for any key type that is not a String
  • and supported nested checks (e.g. "mapName.key")

Would you have a look at it again? Thanks in advance!

Copy link
Copy Markdown
Contributor

@hduelme hduelme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thunderhook really nice improvement. I looked through your implementation. I left two small comments. Otherwise, approved from my side.

}

PsiType[] parameters = ct.getParameters();
if ( parameters.length == 0 ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like in FromMapMappingMapTypeInspection parameters.length should be check against 2, that is nearer on the implementation of MapStruct.

This should catch invalide code like Map<String, String, String>.

public abstract CorporateAction mapWithMultipleSourcesAndMapName(
Map<String, String> rowValues,
LocalDate payDate
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test looks correct.

I think it would be cleaner to split this into separate custom tests. Typically, erroneous and non-erroneous cases aren’t mixed together in the same test.

@thunderhook
Copy link
Copy Markdown
Contributor Author

@thunderhook really nice improvement. I looked through your implementation. I left two small comments. Otherwise, approved from my side.

Thanks for the review! I've adapted the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants